linux: the read-deny glob walk has a budget - #607
Open
ronleizrowice-ant wants to merge 14 commits into
Open
ronleizrowice-ant wants to merge 14 commits into
ronleizrowice-ant wants to merge 14 commits into
Conversation
bubblewrap has no patterns, so a denyRead entry with glob syntax is expanded on the host into the paths to mount over, on every wrap. That walk listed through any symbolic link to a directory, wherever it led; the only link it declined was one leading back up the tree. With a link from a project to /usr and `**/.env` denied, each expansion listed 57,000 entries: 260 to 320 ms per command warm, seconds cold, and no limit for a larger target. A sandboxed command can plant such a link. A link whose target lies outside the pattern's tree is no longer listed through. The tree is the pattern's literal starting directory and what is under it, and a link is judged by where it resolves. Links that stay inside are followed as before, and a link whose own name matches still masks what it leads to. What this gives up is a file the pattern matches only by a name that passes through such a link: macOS never covered it, since patterns are matched against resolved paths there. Where a matched link hides a directory outside the tree that the pattern would have carried on into, nothing is bound back beneath it: it is reported through unlistableDenyDirs like a directory the walk cannot list. Unlisted, an allowed path beneath it would otherwise come back with nothing masked. The walk takes a budget, shared by all the denyRead patterns of one configuration: 2,000,000 directory entries and 10 seconds, the clock read before each listing and at each entry. Out of budget it throws and hands nothing back, and the wrap is refused with LinuxSandboxProfileError `deny_glob_too_large`, the walk's error as its cause. getFsReadConfig() throws the same. A deny list cut short would leave readable what the pattern was written to hide. Each expansion logs one line under SRT_DEBUG: milliseconds, matches, mounts, directories listed, entries looked at, links left unfollowed.
Ten existing cases pinned the listing through a link out of the tree. Five now expect the rule. Five are about something else (a chain of links, a path too long to name, `**` written against text, a bracket expression, a carve-out): they keep their expectations under a base widened to hold the link, and assert the original pattern under the new rule beside it. None is removed. New cases pin the rule from both sides: a link out is never listed, by a count of listings, and a link inside the tree that is the only route to a match is still followed. A link is judged by where it resolves; the tree is the pattern's and not the project's; a matched link still masks its target. Nothing is bound back beneath an unlisted target, shown by a real bubblewrap run in which the file beneath the allowed path is unreadable under both names. The budget: the exact entry boundary, the deadline before and during the walk and inside a small directory, one budget shared by the patterns of a wrap, never a partial result. At the manager the wrap rejects and returns no command, the getter throws the same, and the fields of the cause are what a caller words its own message from.
Open
…llowed A link out of a denyRead pattern's tree is not listed through, and what the pattern would have matched beneath it is not denied. That was recorded on the walk and written to the debug log; a caller had no way to tell its user. getFsReadConfig() now lists each such link in `unfollowedDenyLinks`, with the pattern that came to it, the link and where it resolves. It informs and restricts nothing: no backend reads it. The type is exported as UnfollowedDenyLink.
It lists the links left unfollowed for leading out of a pattern's tree. A link that leads back up the tree, and any link under a pattern that cannot be followed one path component at a time, is not listed through either and is not in the list. The field's comment and the README read as if the list were of every link not followed.
Where a link whose own name matches hides a directory outside the pattern's tree, the expansion reported that directory as one it could not list, so that nothing was bound back beneath it. The wrapper then restores no allowed path at or beneath it, the paths that are always writable included: with a link to a directory above the command's temporary directory, every command failed, and an allowed write path beneath such a directory stopped applying. Through the ancestor that hides it, a second link could take the working directory along. The report is taken out again. A directory outside the tree is never listed, whether or not the link that leads there matches and whatever is allowed beneath it; a matched link still hides its target whole; an allowed path beneath it is bound back as written, as beneath a directory denied literally. `unlistableDenyDirs` again names only a directory the walk could not list, and the code that fills it is what it was. What that gives up is what such a link used to add: what the pattern matched beneath the allowed path through the link's name is not masked again. A link out of the tree can only add to what a pattern covers. A file whose real path is in the tree and matches is found by walking the tree itself, so no link a sandboxed command creates takes a mask away. The unfollowed links are handed back sorted by name, so that the list and the debug line do not depend on the order a directory is read in.
The cases written for the report of an unlisted directory now state the rule as it is: what a matched link out of the tree leads to is hidden whole and not listed, with nothing allowed beneath it and with an allowed path at it, beneath it or written through the link; nothing is handed back as unlistable; a link inside such a directory is never come to. Two real bubblewrap runs pin what follows from it: beneath such a directory an allowed path is readable and the rest stays hidden, under either name, and a write allowed there succeeds. The unfollowed links come back in the same order whichever way the directory is read.
The walk stopped at a symbolic link that leaves the pattern's tree. Every release follows such a link and masks what it finds behind it: with `project/out` a link to `../outside` and `project/**/.env` denied, `outside/deep/.env` is unreadable under both names on 0.0.75, 0.0.76 and 0.0.77. Stopping there left it readable, so the rule lowered what a pattern covers and is taken out. The messages of the earlier commits on this branch that describe it as what other releases and platforms do were wrong on that point. Link handling in walkGlobPattern is what it was. `GlobWalk.unfollowedLinks`, the out-parameter of expandReadDenyGlobLinux, and `unfollowedDenyLinks` with its exported type are removed. What stays is what the walk never had: a budget shared by all the denyRead patterns of one configuration, 2,000,000 directory entries and 10 seconds, the clock read before each listing and at each entry. Out of budget the walk throws and hands nothing back, the wrap is refused with LinuxSandboxProfileError `deny_glob_too_large`, and getFsReadConfig() throws the same. What lies behind a link counts against it like anything else. Each expansion logs one line under SRT_DEBUG with what it cost.
…budget The two existing glob suites are as they were before this branch. The new file keeps the cases for the budget, for the refusal at the manager and for the debug line, and loses the ones written for the rule. Added: a match behind a link to a directory beside the project is denied where it really is, and a real bubblewrap run shows it unreadable under both names; what such a link leads to is spent from the budget, at the walk and at the manager.
Comments only: the JavaScript emitted with comments removed is byte-identical for every file, and the test names are unchanged. Kept: every invariant and what could be done without it, the documentation of exported names, and the reasons for what is not done the obvious way. Cut: how the code came to be as it is, narration of the lines that follow, and explanations repeated at several sites.
…to change it The default of 2,000,000 entries and 10 seconds refused every command in a large monorepo: one `**/.env` pattern over 2.5 million entries walks in about 7 seconds. Such a user waited before; refused, they could not run anything, and nothing could raise the limit. The defaults are now 20,000,000 entries and 60 seconds, and `filesystem.denyReadGlobBudget` (`maxEntries`, `timeoutMs`) sets others, from the initialized configuration, updateConfig() or one wrap's customConfig. The refusal names the option. README: wrapWithSandboxArgv() rejects the same way; the patterns share one deadline; an over-budget configuration is accepted at initialize() and every wrap of it spends the budget again.
A misspelt `maxEntires` was accepted and ignored, and the default applied in silence.
This was referenced Sep 28, 2026
ronleizrowice-ant
added a commit
that referenced
this pull request
Sep 30, 2026
Both it and the walk's budget (#607) hand every pattern of one read configuration something to share, so the listings go where the budget already is: made in readDenyGlobExpander(), and handed on in the options of expandReadDenyGlobLinux(). The walk's record of a directory gives up its own copy of the entries for the shared map. An entry is paid for before it is skipped: the budget bounds the entries looked at, and one the fast path passes over was looked at too.
ronleizrowice-ant
added a commit
that referenced
this pull request
Sep 30, 2026
The walk's budget (#607), its shared listings (#627) and its steps (#628) rewrite one code path, and the path entries of #620 to #622 sit on top of it. Everything above the walk is a generator now: the read-deny expansion, the function readDenyGlobExpander() returns, the allowRead expansion, the resolution of read path entries, and withOtherReadings(), which calls the expander once for each reading of an entry. The synchronous names finish those on the spot. In the walk the step comes first and the budget's clock is read after it, before the next listing. The deadline is wall time, so turns given to other work count against it: a wrap can be refused earlier for that, and a list is never cut short. The budget error is given its code around the delegated generator. An abort is raised by the driver and does not pass through there, so a wrap with a spent budget and an aborted signal rejects with the signal's reason. A wrap that starts over makes a new budget and new listings, and reads the new configuration's denyReadGlobBudget. In the scan's catch the abort is looked at first, then what ripgrep listed.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it does
On Linux the
denyReadglobs of one configuration are expanded on one shared budget: directory entries looked at, and wall time. When it runs out,wrapWithSandbox(),wrapWithSandboxArgv()andgetFsReadConfig()fail withLinuxSandboxProfileError, codedeny_glob_too_large. Nothing shortened is returned.SRT_DEBUGlogs what each expansion cost.The first commit's message describes a rule (stopping at links out of the tree) that the branch no longer has: links are followed as before.
Defaults and option
20,000,000 entries and 60,000 ms.
filesystem.denyReadGlobBudget: { maxEntries?, timeoutMs? }(positive integers, no other key) sets others, frominitialize(),updateConfig()or one wrap'scustomConfig. Only the schema checks the values, and like every other option they are not checked again on the last two routes. A monorepo of 2.5 million entries walks in about 7 s warm and fits.Who sees a difference
Only a configuration whose patterns walk past the budget: it used to wait, now it is refused. Each
**pattern walks its tree again.Known limits
initialize()andupdateConfig()accept an over-budget configuration, and nothing is cached: every refused wrap spends the budget again, synchronously. One blocking filesystem call or one slow match is not cut short. All patterns share one deadline.Merge order
Not independent: textual conflicts with #569 (README ~748, linux-sandbox-utils.ts ~753-758, sandbox-manager.ts imports), #573 (README ~442), #584 and #615 (README ~749, linux-sandbox-utils.ts ~763-770), #594 (linux-sandbox-utils.ts ~753-764), #575 (README ~442, sandbox-utils.ts ~1644-1662 and ~1723-1739, plus an unmarked break:
listings.sizeis gone after #575, andrecords.sizeis no substitute: it also counts directories that could not be listed) and #608 (five hunks; a resolution that drops #608'sanchorargument type-checks and silently loses it; anchored walks need the budget too). Whoever merges second resolves by hand.